src/ws_transport.zig (3)

1-1164: ⚠️ Potential issue | 🟡 Minor

zig fmt --check is still failing on this file.

CI is blocked until src/ws_transport.zig is reformatted with zig fmt.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ws_transport.zig` around lines 1 - 1164, The file fails zig fmt checks;
run the formatter and commit the changes so CI passes—specifically run `zig fmt
src/ws_transport.zig` (or `zig fmt --write`/IDE formatter) to reformat the
entire file including top-level declarations like parseUrl, encodeFrame,
decodeFrameHeader, buildHandshakeRequest, computeAcceptKey, WsTransport and its
methods (connect, initTls, performHandshake, sendFrame, readFrame, writeAll,
readSome). Ensure no manual code changes other than whitespace/formatting are
made and re-run `zig fmt --check` to verify.

---

44-59: ⚠️ Potential issue | 🟠 Major

Handle valid WebSocket URLs with IPv6 and query-only targets.

This authority split regresses on valid URLs. std.mem.indexOfScalar(u8, host_port, ':') breaks ws://[::1]:8545/, and splitting the path only on '/' treats ws://host?token=abc as part of the host instead of preserving /?token=abc.

---

442-448: ⚠️ Potential issue | 🟠 Major

Substring ID matching can return the wrong JSON-RPC response.

Matching on "id":{d} is not exact: waiting for id = 1 will also match "id":12, and valid JSON like "id": 1 is missed. This can either return another request's response or keep looping forever depending on formatting. Parse the envelope and compare the id field exactly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ws_transport.zig` around lines 442 - 448, The current substring match
using id_buf/id_str and std.mem.indexOf against frame_data can produce false
positives (e.g. id=1 matching "id":12) or miss spaced JSON; instead parse the
incoming envelope and compare the parsed "id" field for exact numeric equality
to the expected id. Replace the std.mem.indexOf check with a JSON parse of
frame_data (using Zig's std.json parser or equivalent), extract the top-level
"id" value and compare it numerically to the expected id, returning frame_data
only on an exact match and handling parse errors/absent id appropriately.

src/mnemonic.zig (1)

175-199: ⚠️ Potential issue | 🟠 Major

Guard the fixed-size buffers before copying.

toSeed still writes into mnemonic_buf and salt_buf without validating capacity first. A passphrase longer than 248 bytes already trips this, and oversized word slices can do the same for mnemonic_buf.

Suggested fix

 pub fn toSeed(words: []const []const u8, passphrase: []const u8) ![64]u8 {
     const secureZero = `@import`("utils/constants.zig").secureZero;

     // Build mnemonic string: words joined by spaces
     var mnemonic_buf: [1024]u8 = undefined;
     defer secureZero(&mnemonic_buf);
     var mnemonic_len: usize = 0;

     for (words, 0..) |word, i| {
+        if (i > 0 and mnemonic_len == mnemonic_buf.len) return error.InputTooLong;
+        const available = mnemonic_buf.len - mnemonic_len - `@as`(usize, `@intFromBool`(i > 0));
+        if (word.len > available) return error.InputTooLong;
         if (i > 0) {
             mnemonic_buf[mnemonic_len] = ' ';
             mnemonic_len += 1;
         }
         `@memcpy`(mnemonic_buf[mnemonic_len .. mnemonic_len + word.len], word);
@@
     var salt_buf: [256]u8 = undefined;
     defer secureZero(&salt_buf);
     const prefix = "mnemonic";
+    if (passphrase.len > salt_buf.len - prefix.len) return error.InputTooLong;
     `@memcpy`(salt_buf[0..prefix.len], prefix);
     `@memcpy`(salt_buf[prefix.len .. prefix.len + passphrase.len], passphrase);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mnemonic.zig` around lines 175 - 199, The code builds fixed-size buffers
mnemonic_buf and salt_buf without bounds checks and can overflow when words or
passphrase are too large; update the toSeed logic to validate sizes before
copying: check each word.len will fit into mnemonic_buf (use mnemonic_len +
word.len <= mnemonic_buf.len) and ensure the passphrase length plus prefix fits
salt_buf (prefix.len + passphrase.len <= salt_buf.len); if a check fails, return
an appropriate error (or slice an error result) instead of writing, and keep the
existing defer secureZero calls; reference mnemonic_buf, mnemonic_len, salt_buf,
prefix, words, and passphrase when applying these checks.

src/secp256k1_c.zig (1)

32-33: ⚠️ Potential issue | 🟡 Minor

Declare and call secp256k1_context_destroy() on the race-losing context instead of leaking it.

The libsecp256k1 library provides secp256k1_context_destroy() (defined in secp256k1.h), so the losing ctx can be properly freed rather than intentionally leaked. The comment claiming there is no destroy function in the API is incorrect.

Suggested fix

 extern fn secp256k1_context_create(flags: c_uint) ?*secp256k1_context;
+extern fn secp256k1_context_destroy(ctx: *secp256k1_context) void;
 extern fn secp256k1_context_randomize(ctx: *secp256k1_context, seed32: ?[*]const u8) c_int;
@@
     // Use cmpxchg to avoid TOCTOU race: if another thread won, use their context
     if (`@cmpxchgStrong`(?*secp256k1_context, &global_ctx, null, ctx, .release, .acquire)) |existing| {
-        // Another thread already initialized; we leak our context (no destroy in libsecp256k1 static API),
-        // but this is harmless and only happens once.
         _ = existing;
+        secp256k1_context_destroy(ctx);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/secp256k1_c.zig` around lines 32 - 33, The code currently omits and
therefore never calls the libsecp256k1 cleanup function; add an extern
declaration for secp256k1_context_destroy(ctx: *secp256k1_context) void
alongside the existing externs (secp256k1_context_create and
secp256k1_context_randomize) and replace the intentional leak by calling
secp256k1_context_destroy on the race-losing ctx instead of leaving it
allocated; ensure the winning ctx remains in use and only the losing ctx is
passed to secp256k1_context_destroy.

src/secp256k1.zig (1)

277-337: ⚠️ Potential issue | 🟠 Major

Zero the HmacSha256 work buffers too.

v and k are scrubbed now, but every hmac_* local in this function still keeps key-derived state on the stack until the frame unwinds. HmacSha256 structs contain both the outer key pad and buffered SHA256 state—both sensitive. This leaves the RFC6979 cleanup incomplete.

Suggested pattern

-    var hmac_d = HmacSha256.init(&k);
-    hmac_d.update(&v);
-    hmac_d.update(&[_]u8{0x00});
-    hmac_d.update(&private_key);
-    hmac_d.update(&message_hash);
-    hmac_d.final(&k);
+    {
+        var hmac_d = HmacSha256.init(&k);
+        defer secureZero(std.mem.asBytes(&hmac_d)[0..]);
+        hmac_d.update(&v);
+        hmac_d.update(&[_]u8{0x00});
+        hmac_d.update(&private_key);
+        hmac_d.update(&message_hash);
+        hmac_d.final(&k);
+    }

Apply the same wrapper to hmac_e, hmac_f, hmac_g, hmac_h2, hmac_k2, and hmac_v2. (Note: slice the asBytes result with [0..] to match the secureZero signature.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/secp256k1.zig` around lines 277 - 337, The HmacSha256 local instances
(hmac_d, hmac_e, hmac_f, hmac_g, hmac_h2, hmac_k2, hmac_v2) retain key material
on the stack and must be zeroed after use; for each HmacSha256 variable in this
function, call secureZero on its byte view (use hmacX.asBytes()[0..] to match
secureZero's parameter) via defer immediately after initialization so their
internal buffers are scrubbed when the scope exits.

src/abi_decode.zig (1)

88-95: ⚠️ Potential issue | 🔴 Critical

Empty dynamic arrays are still treated as owned allocations.

decoded_count fixes the uninitialized-prefix cleanup, but a decoded empty array still lands in result[0..decoded_count]. decodeDynamicArray() returns .array = &.{} for length == 0, while freeValue(.array) always calls allocator.free(items), so a valid empty array will still trigger an invalid free during cleanup or freeValues().

Suggested fix

 fn decodeDynamicArray(data: []const u8, offset: usize, element_type: AbiType, allocator: std.mem.Allocator) DecodeError!AbiValue {
     if (offset + 32 > data.len) return error.OffsetOutOfBounds;

     const length = readUint256AsUsize(data[offset..][0..32]);
     const elements_start = offset + 32;

     if (length == 0) {
-        const empty: []const AbiValue = &.{};
-        return .{ .array = empty };
+        return .{ .array = try allocator.alloc(AbiValue, 0) };
     }

Also applies to: 110-110

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/abi_decode.zig` around lines 88 - 95, The cleanup/free path incorrectly
frees static empty-array slices returned by decodeDynamicArray() (the &.{ }
case), causing invalid frees during the errdefer or freeValues(); update the
logic so empty arrays are not treated as owned allocations: either change
decodeDynamicArray() to return a null/owned-empty pointer for length == 0, or
(simpler) modify freeValue() / freeValues() to skip allocator.free(items) when
the slice length is 0 (check the value's length before calling allocator.free)
so result + decoded_count cleanup won't free the static &.{ } slice. Ensure you
reference and update decodeDynamicArray(), freeValue(), freeValues(), and the
errdefer cleanup that iterates result[0..decoded_count].

src/multicall.zig (1)

169-186: ⚠️ Potential issue | 🔴 Critical

Only clean up the initialized prefix of results.

results is uninitialized after allocator.alloc. Any return error.InvalidAbiData inside the loop before results[i] is assigned makes the current errdefer read garbage return_data slices from the remaining slots and free them.

Suggested fix

     var results = try allocator.alloc(Result, array_len);
+    var parsed_count: usize = 0;
     errdefer {
-        for (results) |r| {
+        for (results[0..parsed_count]) |r| {
             if (r.return_data.len > 0) allocator.free(r.return_data);
         }
         allocator.free(results);
     }
@@
         results[i] = .{
             .success = success_word != 0,
             .return_data = return_data,
         };
+        parsed_count += 1;
     }

Also applies to: 189-222

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/multicall.zig` around lines 169 - 186, The errdefer in
decodeAggregate3Results currently assumes every element of the allocated results
slice is initialized and frees their return_data, which can read uninitialized
memory; fix by tracking how many entries are successfully initialized (e.g., a
var initialized: usize = 0) and increment it immediately after assigning
results[i] (or after allocating/setting results[i].return_data), then change the
errdefer to only iterate results[0..initialized] when freeing return_data and
finally free the results buffer; apply the same initialized-count pattern to the
other similar function(s) mentioned (the code around the second block at lines
~189-222) to avoid freeing uninitialized slots.

src/abi_json.zig (1)

217-246: ⚠️ Potential issue | 🟠 Major

parseType still accepts malformed ABI type strings.

uint257, int999, and bytes33 still collapse to valid scalar types because these branches use orelse defaults, and uint257[]/custom_type[2] are accepted even earlier by the array fast-path. That means the new nullable API still won't surface error.UnknownType for several invalid ABI strings.

Suggested fix

 pub fn parseType(type_str: []const u8) ?AbiType {
     // Handle array suffixes
-    if (std.mem.endsWith(u8, type_str, "[]")) return .dynamic_array;
+    if (std.mem.endsWith(u8, type_str, "[]")) {
+        _ = parseType(type_str[0 .. type_str.len - 2]) orelse return null;
+        return .dynamic_array;
+    }

     // Check for fixed array (e.g., "uint256[3]")
-    if (type_str.len > 0 and type_str[type_str.len - 1] == ']') return .fixed_array;
+    if (type_str.len > 0 and type_str[type_str.len - 1] == ']') {
+        const open = std.mem.lastIndexOfScalar(u8, type_str, '[') orelse return null;
+        _ = std.fmt.parseInt(usize, type_str[open + 1 .. type_str.len - 1], 10) catch return null;
+        _ = parseType(type_str[0..open]) orelse return null;
+        return .fixed_array;
+    }
@@
     // uint types
     if (std.mem.startsWith(u8, type_str, "uint")) {
-        return parseUintType(type_str) orelse .uint256;
+        return if (type_str.len == 4) .uint256 else parseUintType(type_str);
     }

     // int types
     if (std.mem.startsWith(u8, type_str, "int")) {
-        return parseIntType(type_str) orelse .int256;
+        return if (type_str.len == 3) .int256 else parseIntType(type_str);
     }

     // bytesN types
     if (std.mem.startsWith(u8, type_str, "bytes")) {
-        return parseBytesType(type_str) orelse .bytes;
+        return if (type_str.len == 5) .bytes else parseBytesType(type_str);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/abi_json.zig` around lines 217 - 246, parseType currently accepts
malformed ABI strings because it defaults to .uint256/.int256/.bytes on parse
failures and treats any trailing "[]" or "]" as arrays without validating the
element type; update parseType so array fast-paths validate the inner type (for
"[]": strip the "[]" and recursively call parseType on the base and return null
if that fails; for fixed arrays: locate the '[' of the last bracket, validate
the base portion with parseType and ensure the bracket content is a non-empty
numeric literal, returning null on failure), and stop using orelse defaults—call
parseUintType(type_str), parseIntType(type_str), and parseBytesType(type_str)
and propagate null if they return null instead of falling back to default scalar
kinds.